Reviewable status: 0 of 7 files reviewed, 4 unresolved discussions (waiting on @dan-starkware)

crates/tests-integration/src/integration_test_utils.rs line 72 at r1 (raw file):

}

fn create_consensus_manager_config()

Current name is confusing, as there are other create_X_config that just return a config object, while this variant also returns the channels. Could you please rename?

Code quote:

create_consensus_manager_config

crates/tests-integration/tests/end_to_end_test.rs line 44 at r1 (raw file):

        .expect("listen to broadcasted messages should finish in time");
    });
    join_handle.await.expect("Task should succeed");

Please wrap the "await for something to happen within a timeout" as a utility function (or add a TODO, can include my name as well).

Code quote:

    let join_handle = tokio::spawn(async move {
        tokio::time::timeout(
            LISTEN_TO_BROADCAST_MESSAGES_TIMEOUT,
            listen_to_broadcasted_messages(
                mock_running_system.consensus_proposals_channels,
                &expected_batched_tx_hashes,
            ),
        )
        .await
        .expect("listen to broadcasted messages should finish in time");
    });
    join_handle.await.expect("Task should succeed");

crates/tests-integration/tests/end_to_end_test.rs line 51 at r1 (raw file):

    expected_batched_tx_hashes: &[TransactionHash],
) {
    // TODO(Dan, Guy): retrieve chian ID (from the config?).

Typo

Code quote:

chian

crates/tests-integration/tests/end_to_end_test.rs line 51 at r1 (raw file):

    expected_batched_tx_hashes: &[TransactionHash],
) {
    // TODO(Dan, Guy): retrieve chian ID (from the config?).

I'd suggest modifying IntegrationTestSetup to hold it as a member, and instantiate the value using StorageTestSetup.

Code quote:

(from the config?)

crates/tests-integration/tests/end_to_end_test.rs line 71 at r1 (raw file):

            );
        }
    }

I didn't review this part, I suggest asking someone more familiar with the consensus to do so. If there's no on else I'll dig into this though, please lmk.

Code quote:

    let mut broadcasted_messages_receiver =
        consensus_proposals_channels.broadcasted_messages_receiver;
    let mut received_tx_hashes = vec![];
    while received_tx_hashes.len() < expected_batched_tx_hashes.len() {
        let (message, _broadcasted_message_metadata) = broadcasted_messages_receiver
            .next()
            .await
            .unwrap_or_else(|| panic!("Expected to receive a message from the broadcast topic"));
        if let ProposalPart::Transactions(transactions) = message.unwrap() {
            received_tx_hashes.append(
                &mut transactions
                    .transactions
                    .iter()
                    .map(|tx| tx.calculate_transaction_hash(&chain_id).unwrap())
                    .collect(),
            );
        }
    }

Reviewable status: 0 of 7 files reviewed, 3 unresolved discussions (waiting on @Itay-Tsabary-Starkware)

crates/tests-integration/src/integration_test_utils.rs line 72 at r1 (raw file):

Done.

crates/tests-integration/tests/end_to_end_test.rs line 44 at r1 (raw file):

Done.

crates/tests-integration/tests/end_to_end_test.rs line 51 at r1 (raw file):

Done.

Reviewed 5 of 7 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @dan-starkware)

Reviewed 7 of 7 files at r7, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware, @guy-starkware, and @Itay-Tsabary-Starkware)

crates/tests-integration/tests/end_to_end_test.rs line 39 at r5 (raw file):

I don't see any change, which is fine for this PR, just noting since you wrote "Done"

crates/tests-integration/tests/end_to_end_test.rs line 68 at r5 (raw file):

macros are harder to work with in the IDE, you don't get nice type hints, and jumping to the definition doesn't bring you to the actual function used, still requires more sleuthing. They also make compile times slower.

I prefer functions when there is a 1 to 1 replacement.

Reviewable status: 5 of 13 files reviewed, 1 unresolved discussion (waiting on @guy-starkware, @Itay-Tsabary-Starkware, and @matan-starkware)

crates/tests-integration/tests/end_to_end_test.rs line 39 at r5 (raw file):

I meant ACK

Reviewed 7 of 8 files at r8, 1 of 1 files at r9, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guy-starkware and @Itay-Tsabary-Starkware)

Reviewed 1 of 6 files at r5, 3 of 7 files at r7, 4 of 8 files at r8, 1 of 1 files at r10, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guy-starkware)

Reviewed 1 of 6 files at r5, 2 of 7 files at r7, 1 of 8 files at r8, 10 of 10 files at r11, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware and @guy-starkware)

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware and @guy-starkware)

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @dan-starkware and @guy-starkware)

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @graphite-app[bot] and @guy-starkware)

Dismissed @graphite-app[bot] from a discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @guy-starkware)

Dismissed @guy-starkware from a discussion.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @guy-starkware)